feat(webcodecs): reset the decoder on every keyframe #826
Conversation
📝 WalkthroughWalkthroughThis PR refactors the WebCodecs decoder configuration and state management across AV1, H264, H265, and H26x codecs. The configure() method return type changes from void to VideoDecoderConfig, shifting responsibility from internal decoder configuration to returning configuration objects. AV1 adds configuration state tracking with deferred setup until keyframes. H26x implements a two-step configuration flow with explicit keyframe sequencing and error validation. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant H26x as H26xDecoder
participant VideoDecoder as WebCodecs VideoDecoder
App->>H26x: decode(config packet)
H26x->>H26x: Store merged config + raw data<br/>Set configured = false
App->>H26x: decode(keyframe packet)
alt First frame after config
H26x->>H26x: `#configureAndDecodeFirstKeyframe`()
H26x->>VideoDecoder: configure(mergedConfig)
H26x->>VideoDecoder: decode(EncodedVideoChunk)
H26x->>H26x: Set configured = true
else Subsequent keyframe
alt decodeQueueSize exists
H26x->>VideoDecoder: reset()
H26x->>VideoDecoder: configure(mergedConfig)
H26x->>VideoDecoder: decode(EncodedVideoChunk)
end
end
App->>H26x: decode(delta frame packet)
alt configured
H26x->>VideoDecoder: decode(EncodedVideoChunk type=delta)
else not configured
H26x->>App: throw "Decoder not configured"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
diffray
bot
added
diffray-review-failed
diffray
bot
added
diffray-review-started
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libraries/scrcpy-decoder-webcodecs/src/video/codec/av1.ts:
- Around line 100-123: The decoder can attempt to decode delta frames before
being configured and will throw InvalidStateError; update the logic in the
packet handling flow (after this.#parseConfig(packet.data)) to check that if
packet.keyframe is false and either this.#configured is false or this.#config is
missing, you should throw a clear Error (e.g., "Decoder not configured for delta
frame") or otherwise prevent calling this.#decoder.decode(); reference
this.#parseConfig, this.#config, packet.keyframe, this.#configured and
this.#decoder.decode to locate the spot to add the guard so delta frames never
reach decode() when the decoder is unconfigured.
🧹 Nitpick comments (1)
libraries/scrcpy-decoder-webcodecs/src/video/codec/h264.ts (1)
39-44: Consider addingcodedWidthandcodedHeightfor consistency with H265Decoder.The H265Decoder includes
codedWidthandcodedHeightin the returned config with a comment stating "Microsoft Edge requires explicit size to work." If this Edge requirement also applies to H.264 decoding, the dimensions should be included here as well.Suggested change
return { codec: codec, + codedWidth: croppedWidth, + codedHeight: croppedHeight, hardwareAcceleration: this.#options?.hardwareAcceleration ?? "no-preference", optimizeForLatency: true, };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libraries/scrcpy-decoder-webcodecs/src/video/codec/av1.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h264.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h265.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h26x.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: yume-chan
Repo: yume-chan/ya-webadb PR: 782
File: libraries/scrcpy-decoder-tinyh264/src/decoder.ts:193-196
Timestamp: 2025-08-24T03:50:43.091Z
Learning: In TinyH264Decoder, configuration failures should propagate the actual error up the client pipeline rather than being recovered locally. The decoder is designed to handle mid-stream configuration switches when the video encoder restarts, and subsequent valid config packets will re-configure correctly regardless of current decoder state.
📚 Learning: 2025-08-24T03:50:43.091Z
Learnt from: yume-chan
Repo: yume-chan/ya-webadb PR: 782
File: libraries/scrcpy-decoder-tinyh264/src/decoder.ts:193-196
Timestamp: 2025-08-24T03:50:43.091Z
Learning: In TinyH264Decoder, configuration failures should propagate the actual error up the client pipeline rather than being recovered locally. The decoder is designed to handle mid-stream configuration switches when the video encoder restarts, and subsequent valid config packets will re-configure correctly regardless of current decoder state.
Applied to files:
libraries/scrcpy-decoder-webcodecs/src/video/codec/h264.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h26x.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/av1.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h265.ts
📚 Learning: 2025-08-28T05:42:55.846Z
Learnt from: yume-chan
Repo: yume-chan/ya-webadb PR: 790
File: libraries/media-codec/src/h265.ts:23-24
Timestamp: 2025-08-28T05:42:55.846Z
Learning: In HEVC codec strings per ISO/IEC 14496-15:2017 Section E.3, leading zeros may be omitted in hexadecimal encoding for general_profile_compatibility_flags and constraint flags. Zero-padding is not required and the current hexDigits implementation is correct.
Applied to files:
libraries/scrcpy-decoder-webcodecs/src/video/codec/h264.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h265.ts
📚 Learning: 2026-01-12T06:58:19.519Z
Learnt from: yume-chan
Repo: yume-chan/ya-webadb PR: 827
File: libraries/scrcpy-decoder-webcodecs/src/video/utils/video-decoder-stream.ts:105-120
Timestamp: 2026-01-12T06:58:19.519Z
Learning: In TransformStream implementations, calling controller.error() immediately clears the internal [[flushAlgorithm]] and [[cancelAlgorithm]] slots, which prevents the flush() and cancel() callbacks from being invoked. Therefore, if a decoder’s error callback calls controller.error(), the stream’s cancel() callback will not run later. Treat this as a सच rule: do not rely on error() to trigger cleanup through cancel; instead, perform explicit cleanup and, if needed, initiate cancel() or reject the downstream promise yourself. Apply this guidance broadly to all TransformStream-based code paths (not just this file) and ensure error handling paths explicitly manage resource cleanup and stream termination without assuming cancel will be invoked.
Applied to files:
libraries/scrcpy-decoder-webcodecs/src/video/codec/h264.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h26x.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/av1.tslibraries/scrcpy-decoder-webcodecs/src/video/codec/h265.ts
🧬 Code graph analysis (3)
libraries/scrcpy-decoder-webcodecs/src/video/codec/h26x.ts (2)
libraries/scrcpy-decoder-webcodecs/src/video/codec/type.ts (1)
CodecDecoder(3-5)libraries/adb-daemon-webusb/src/device.ts (1)
raw(241-243)
libraries/scrcpy-decoder-webcodecs/src/video/codec/av1.ts (1)
libraries/scrcpy/src/codec/nalu.ts (1)
Error(214-221)
libraries/scrcpy-decoder-webcodecs/src/video/codec/h265.ts (1)
libraries/scrcpy-decoder-webcodecs/src/video/codec/av1.ts (1)
data(25-93)
🔇 Additional comments (8)
libraries/scrcpy-decoder-webcodecs/src/video/codec/h265.ts (1)
22-52: LGTM! Clean refactor to return configuration.The
configure()method now correctly returns aVideoDecoderConfigobject instead of configuring the decoder directly. This aligns with the new pattern where the base classH26xDecoderhandles decoder configuration and reset logic.Note: H265Decoder includes
codedWidthandcodedHeightin the config (for Microsoft Edge compatibility), while H264Decoder in this PR does not. If Edge also requires explicit dimensions for H.264, consider adding them there as well for consistency.libraries/scrcpy-decoder-webcodecs/src/video/codec/av1.ts (2)
86-93: Every keyframe triggers reconfiguration due to#configured = falsein#parseConfig.Setting
#configured = falseat line 92 means every call to#parseConfig(which happens on every packet) will mark the decoder as unconfigured. Combined with the logic at lines 119-121, this means every keyframe will callthis.#decoder.configure()even when the decode queue is empty.If this is intentional (per the PR title "reset the decoder on every keyframe"), the current implementation achieves this for keyframes but doesn't actually call
reset()whendecodeQueueSizeis 0—it only reconfigures. However, if the intent is to only reset/reconfigure when the queue is backed up, the#configured = falseshould be conditional.Is the intention to reconfigure on every keyframe regardless of queue size, or only when
decodeQueueSize > 0? If the latter, the#configuredflag should only be reset when the config actually changes:Possible fix if only reset on config change
this.#config = { codec, hardwareAcceleration: this.#options?.hardwareAcceleration ?? "no-preference", optimizeForLatency: true, }; - this.#configured = false; + // Only mark as unconfigured if this is a new/changed config + // Or remove this line if reconfiguration should only happen on queue backup }
125-132: LGTM! Clear handling of keyframe type mapping.The non-null assertion on
packet.keyframe!is appropriate given the documented requirement that AV1 streams from Scrcpy 2.0+ always include the keyframe flag. The comment clearly explains this assumption.libraries/scrcpy-decoder-webcodecs/src/video/codec/h26x.ts (5)
11-12: Good addition of configuration state tracking.The
#configfield stores the video decoder config along with the raw configuration data needed for Annex B concatenation. The#configuredflag properly tracks whether the decoder has been configured, enabling the reset-on-keyframe pattern.
20-43: LGTM! Correct Annex B handling for H.264/H.265.The helper correctly implements the WebCodecs AVC codec registration requirement where configuration data must be combined with the first frame data for Annex B format streams. The reference to the W3C spec is helpful.
59-72: LGTM! Effective latency-limiting reset strategy.The reset logic correctly identifies when the decode queue is backed up and resets on the next keyframe. The comment clearly explains the rationale—limiting maximum latency to one keyframe interval (60 frames by default). This aligns with the PR objective.
80-90: Good backward compatibility for legacy Scrcpy versions.The handling of
packet.keyframe === undefinedfor Scrcpy <1.23 is a thoughtful addition. The comment explains the behavior, and the fallback to treating the first frame as a keyframe (with VideoDecoder validating) is pragmatic. The explicit error for modern Scrcpy receiving delta frames before keyframes provides clear feedback.
92-99: LGTM! Correct frame type handling.The ternary
packet.keyframe === false ? "delta" : "key"correctly treatsundefinedaskeyfor backward compatibility with older Scrcpy versions, as documented in the comment.
Summary by CodeRabbit
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.